Refactor register shutdown function mechanism #5829
Conversation
Girgias
force-pushed
the
refactor-shutdown-function
branch
from
728d666 to
0ae973d
Compare
Girgias
force-pushed
the
refactor-shutdown-function
branch
2 times, most recently
from
5703a5c to
ddb78c5
Compare
trowski
force-pushed
the
refactor-shutdown-function
branch
from
ddb78c5 to
4917e47
Compare
Use FCI/FCC structure instead of custom implementation which does the same. This also fixes the "bug" which prevented static methods from being shutdown functions. Co-authored-by: Aaron Piotrowski <aaron@trowski.com>
Girgias
force-pushed
the
refactor-shutdown-function
branch
from
da80874 to
26d2ac7
Compare
| ZEND_PARSE_PARAMETERS_START(1, -1) | ||
| Z_PARAM_FUNC(entry.fci, entry.fci_cache) | ||
| Z_PARAM_VARIADIC('+', params, param_count); | ||
| ZEND_PARSE_PARAMETERS_END(); |
There was a problem hiding this comment.
Should we keep the non Fast ZPP variant?
There was a problem hiding this comment.
That would make sense to me, though I don't care strongly. At least this should be using '*' insteadof '+' though (even though this is not actually used).
| ZEND_PARSE_PARAMETERS_START(1, -1) | ||
| Z_PARAM_FUNC(entry.fci, entry.fci_cache) | ||
| Z_PARAM_VARIADIC('+', params, param_count); | ||
| ZEND_PARSE_PARAMETERS_END(); |
There was a problem hiding this comment.
That would make sense to me, though I don't care strongly. At least this should be using '*' insteadof '+' though (even though this is not actually used).
| result = zend_fcall_info_init(&callable, 0, &shutdown_function_entry.fci, | ||
| &shutdown_function_entry.fci_cache, NULL, NULL); | ||
|
|
||
| ZEND_ASSERT(result == SUCCESS); |
There was a problem hiding this comment.
This change will break some extensions, it would be better to mention it in the internal changelog :)
In addition, if ZEND_ASSERT is empty, this may cause an unused variable warning, right?
There was a problem hiding this comment.
How can this break an extension, as if it's calling an internal function not part of itself it should declare it as a dependency.
I wouldn't worry about the compiler warning as I would expect for this to be understood by the preprocessor that it is used initially just in an empty macro, which can be optimized out.
There was a problem hiding this comment.
as if it's calling an internal function not part of itself it should declare it as a dependency.
I tried but I can not understand...
I mean extensions need to change the code because the initialization way has changed.
In fact, there are many ZEND_API break changes now... We often receive notifications from CI that the extension built failed with php-master. But I believe all of the changes are pretty good, it just makes a little trouble for extension developers... Maybe we need to get used to these, so, forget this.
Use a more sane mechanism than what is currently in place.